Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update docs #542

Merged
merged 4 commits into from
Dec 13, 2023
Merged

Update docs #542

merged 4 commits into from
Dec 13, 2023

Conversation

rxu17
Copy link
Contributor

@rxu17 rxu17 commented Dec 11, 2023

Purpose: Just a small PR to update documentation to be consistent with the logic of the pipeline.

Changes Made

  • Updates to README.md to include the up to date logic in running the pipeline steps (Note that there is a JIRA ticket to discuss the current logic here)
  • Updates to CONTRIBUTING.md to include section for running/testing docker, running pipeline steps on test pipeline

@rxu17 rxu17 requested a review from a team as a code owner December 11, 2023 23:54
Copy link
Member

@thomasyu888 thomasyu888 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 nice! Just a small comment about using env variable to log into the client instead of the config file.

See the authentication portion of synapseclient docs.

CONTRIBUTING.md Outdated

Follow this section when modifying the [Dockerfile](https://github.com/Sage-Bionetworks/Genie/blob/main/Dockerfile):

1. Make sure you have your synapse config setup in your working directory
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your rationale of doing this over using env variable to log into the python client?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No rationale actually, I think I just carried this over from another project (specifying the env variable would be easier). Let me modify the docker command so it only has relevant flags in the command.

@rxu17 rxu17 merged commit c01f67d into develop Dec 13, 2023
7 checks passed
@rxu17 rxu17 deleted the update-docs branch December 13, 2023 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants